-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make the cdylib no_std #59
Conversation
Codecov ReportAttention: Patch coverage is
|
a65d24d
to
75fcfe0
Compare
The default features includes both c-allocator and (through std) rust-allocator. Rust-allocator wins when both are given, so remove c-allocator from the default features.
This reduces the size of libbz2_rs.so by 408KB in release mode.
.github/workflows/checks.yaml
Outdated
- name: "cdylib: no_std" | ||
env: | ||
LD_LIBRARY_PATH: "target/${{matrix.target}}/release/deps" | ||
working-directory: libbzip2-rs-sys-cdylib | ||
run: | | ||
cargo build --release --target ${{matrix.target}} --no-default-features | ||
cc -DNO_STD -o bzpipe bzpipe.c target/${{matrix.target}}/release/deps/libbz2_rs.so -I ../ | ||
./bzpipe < Cargo.toml | ./bzpipe -d > out.txt | ||
cmp -s Cargo.toml out.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so now we never build wit h-DNO_STD
right? we just always use the c allocator; in practice that means we test less (because the rust allocator was also dropped).
Why make that change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this example is not really practical, it's just for demonstration and testing purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add back a -DNO_STD
C example, but with this PR the cdylib will always be compiled with #![no_std]
, which implies that we always have to use the C allocator as the Rust allocator is part of libstd. There is no benefit to building the cdylib with std. It only adds extra work and increases the size of the dylib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well the NO_STD
flag is maybe misnamed then, but I'd like to test the use of custom allocators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added back testing of the no-stdio mode.
No description provided.